Review

Arkade script support — comprehensive implementation ✓

Scope: Full client-side Arkade scripting (opcodes, introspector, banking)

Key observations:

1. Opcode design (0xc4–0xf2) — correctly uses available Bitcoin opcodes for Arkade extension; ASM roundtrip and codec solid.

2. Key tweaking — ArkScriptHash-based public key point addition follows standard tagged hash pattern. Verify cross-repo compatibility with introspector's expectations.

3. Contract system — DefaultContractHandler + DelegateContractHandler separation is good; VTXO multi-script support (contract-aware) properly handles delegate mode transitions.

4. Introspector integration — REST client + batch handler looks correct, though ensure error handling for intent submission timeouts.

5. PSBT fields — New ArkadeScript unknown fields properly avoid colliding with standard Bitcoin fields. Type-safe key matching in decoders ✓

6. Tests — E2E against local Ark + introspector is strong coverage. Ensure docker-compose.yml versions pin introspector & arkd commits.

Minor items:

• Document CSV lock behavior differences between DefaultVtxo (forfeit gate) and DelegateVtxo (3-leaf)
• Confirm SIGHASH_ALL|ANYONECANPAY delegation flow aligns with arkd expectations

Ship it — protocol-sound, well-tested.

Banco Partial Fill Spec

Overview

Extend the banco swap contract so a taker can fill part of an offer. The script has two hardcoded constants — an exchange ratio (expressed as a fraction ratioNum / ratioDen) and the maker's taproot address. On each partial fill the maker receives some asset X, and the remaining BTC is recycled back into an identical script for further fills.

Ratio as Integer Arithmetic

The stack has no floating-point support. The ratio is encoded as two integers — a numerator and denominator:

The consumed BTC for a given fill is computed entirely with 64-bit integer ops:

consumed = (received × ratioNum) / ratioDen        ← integer division, rounds down
change   = input_value − consumed

Using OP_MUL64 then OP_DIV64. OP_DIV64 pushes both the quotient and remainder; the remainder is discarded (OP_NIP). Any rounding dust stays in the change VTXO — safe for the maker.

Example ratios:

Worked Example

Setup. Alice (maker) locks 1 BTC (100 000 000 sats) wanting 5 units of Asset X.
ratioNum = 100 000 000, ratioDen = 5.

Fill 1 — Bob delivers 2 X (partial)

Inputs                              Outputs
───────────────────                 ─────────────────────────────
[0] swap VTXO  (1 BTC, banco)      [0] banco script: 0.6 BTC     ← change (same script)
[1] Bob VTXO   (has 2 X)           [1] Alice: 2 X                ← maker gets asset
                                    [2] Bob: 0.4 BTC              ← taker (unconstrained)

Script enforces:

1. Output 1 pays Alice's taproot address and carries 2 units of Asset X
2. consumed = 2 × 100 000 000 / 5 = 40 000 000 sats
3. consumed (40M) < input_value (100M) → partial fill
4. change = 100 000 000 − 40 000 000 = 60 000 000
5. Output 0 value == 60 000 000 ✓
6. Output 0 scriptPubKey == input scriptPubKey (same banco script) ✓

Fill 2 — Carol delivers 3 X (full fill)

Inputs                              Outputs
───────────────────                 ─────────────────────────────
[0] change VTXO (0.6 BTC, banco)   [0] Carol: 0.6 BTC            ← taker (unconstrained)
[1] Carol VTXO  (has 3 X)          [1] Alice: 3 X                ← maker gets asset

Script enforces:

1. Output 1 pays Alice's taproot address, carries 3 X
2. consumed = 3 × 100 000 000 / 5 = 60 000 000
3. consumed (60M) ≥ input_value (60M) → full fill
4. No constraints on output 0 — Carol takes the 0.6 BTC however she wants.

Output Layout

Script: partialFillScript

Handles both partial and full fills. Hardcoded into the script bytes:

Pseudocode with stack trace

# ═══════════════════════════════════════════════════
# STEP 1 — Verify output 1 pays the maker
# ═══════════════════════════════════════════════════
OP_1                                   # [1]
OP_INSPECTOUTPUTSCRIPTPUBKEY           # [program, version]
OP_1 OP_EQUALVERIFY                    # [program]            ← taproot v1
<makerWitnessProgram> OP_EQUALVERIFY   # []                   ← correct key

# ═══════════════════════════════════════════════════
# STEP 2 — Read asset X amount from output 1
# ═══════════════════════════════════════════════════
OP_1 <assetTxid> <groupIdx>           # [1, txid, gidx]
OP_INSPECTOUTASSETLOOKUP              # [amount_le64]         (or -1 if absent)
OP_DUP OP_1NEGATE OP_EQUAL
OP_NOT OP_VERIFY                       # [amount_le64]         ← verified found

# ═══════════════════════════════════════════════════
# STEP 3 — Compute consumed BTC = amount × ratioNum / ratioDen
# ═══════════════════════════════════════════════════
<ratioNum_le64> OP_MUL64 OP_VERIFY    # [product_le64]
<ratioDen_le64> OP_DIV64 OP_VERIFY    # [remainder, quotient]
OP_NIP                                 # [consumed_le64]       ← drop remainder

# ═══════════════════════════════════════════════════
# STEP 4 — Get input value (BTC locked in this VTXO)
# ═══════════════════════════════════════════════════
OP_PUSHCURRENTINPUTINDEX               # [consumed, idx]
OP_INSPECTINPUTVALUE                   # [consumed, input_val]

# ═══════════════════════════════════════════════════
# STEP 5 — Full fill or partial fill?
# ═══════════════════════════════════════════════════
OP_2DUP                               # [consumed, input_val, consumed, input_val]
OP_SWAP                               # [consumed, input_val, input_val, consumed]
OP_LESSTHANOREQUAL64                  # [consumed, input_val, flag]
                                       #   flag=1 → input_val ≤ consumed (full fill)
                                       #   flag=0 → input_val > consumed (partial fill)

OP_IF
  # ── FULL FILL ──────────────────────
  OP_2DROP                             # []
  OP_1                                 # [1]  ← success, no output constraints

OP_ELSE
  # ── PARTIAL FILL ───────────────────
  # change = input_value − consumed
  OP_SWAP                              # [input_val, consumed]
  OP_SUB64 OP_VERIFY                   # [change_le64]

  # 5a. output 0 value == change
  OP_0 OP_INSPECTOUTPUTVALUE           # [change, out0_val]
  OP_EQUALVERIFY                       # []

  # 5b. output 0 script == current input's script
  OP_PUSHCURRENTINPUTINDEX
  OP_INSPECTINPUTSCRIPTPUBKEY          # [in_prog, in_ver]
  OP_0
  OP_INSPECTOUTPUTSCRIPTPUBKEY         # [in_prog, in_ver, out_prog, out_ver]
  OP_ROT                               # [in_prog, out_prog, out_ver, in_ver]
  OP_EQUALVERIFY                       # [in_prog, out_prog]
  OP_EQUAL                             # [match]  ← success

OP_ENDIF

Review: Arkade script support

✅ Major feature — comprehensive implementation

This PR adds introspector-based smart contract support to the SDK. 46 files is large, so I'll focus on protocol correctness:

Script & Opcode Safety:

• Arkade opcodes (0xc4–0xf2) don't conflict with future Bitcoin opcodes (OP_NOP1..OP_NOP10 reserved space)
• Tagged hash for key tweaking (ArkScriptHash) — verify this matches introspector's expected hash domain separator
• Script encoding handles both standard + Arkade opcodes; verify edge cases (empty scripts, invalid opcode sequences)

Introspector Integration:

• REST client submits intents to introspector → introspector co-signs using derived key
• Who validates the introspector's signature on the co-signed intent? (Should be SDK before broadcast)
• Fallback behavior if introspector is unavailable during settlement?

Contract Lifecycle:

• ContractManager tracks active contracts; verify cleanup on completion (prevent memory leaks)
• Batch handler merges boarding + settlement — does it handle partial failures? (one script fails, others succeed?)

Docker Setup:

• introspector.Dockerfile — good for local dev. Document how to connect SDK against mainnet introspector (if public instance exists)

Tests:

• 46 files → many unit tests, good. E2E tests against local Ark + introspector stack are essential. Verify they pass in CI.

Cross-repo coordination:

• ts-sdk, go-sdk, dotnet-sdk, rust-sdk will all need Arkade support to be feature-complete. Any coordinated release timeline?

✅ Mergeable — structure is solid, but verify introspector signature validation in code review

🔍 Review: fix banco fullfill (fa18888)

Iterative review — focused on the new commit since last pass.

What changed

Major rework of the Banco taker fulfillment flow and offer serialization:

1. BancoSwap.fromOffer() factory — DRYs up the duplicated swap construction in maker.ts and taker.ts. Good refactor.

2. Offer.wantAsset type: string → AssetId — Binary TLV serialization instead of UTF-8 "txid:vout". Breaking change to the wire format but correct direction — matches the rest of the extension system.

3. Taker coin selection overhaul — Proper target-based selection via selectVirtualCoins / selectCoinsWithAsset instead of naively using all VTXOs. Handles asset swaps, BTC swaps, dust thresholds, collateral assets, and BTC change correctly.

4. Asset fulfillment script fix — INSPECTOUTASSETLOOKUP stack args corrected: txid now in internal (LE) byte order, output index hardcoded to 0, and added SCRIPTNUMTOLE64 before the comparison. Critical fix — the previous version would have failed at script validation.

5. Checkpoint merging — Replaced manual tapScriptSig splicing with combine(). Cleaner and preserves taproot metadata the server strips.

6. Wallet extensions support — Recipient.extensions field lets callers embed custom extension packets. Nice for composability.

7. New E2E test — Asset-for-asset swap (maker sells asset A, wants asset B).

Security notes

• Offer contract verification ✅ — Taker reconstructs the swap contract from offer fields and verifies swapPkScript === expectedPkScript before proceeding. Good.
• Introspector signature check ✅ — introInput0.tapScriptSig is verified non-empty before submitting to the server.
• Coin selection ✅ — Properly handles insufficient funds, dust threshold (450 sats), and asset change routing.

Observations

1. DUST = 450n hardcoded in taker — The wallet itself likely has a dust constant; consider importing it for consistency. If the server's dust threshold changes, this would silently diverge.

2. fulfillScript() txid byte order comment is valuable — The LE reversal for chainhash.Hash compatibility is a common footgun. Good that it's documented inline.

3. Collateral asset tracking — The collateral map correctly avoids double-counting the wanted asset and preserves unrelated assets. Edge case: if a taker coin carries the same asset as the swap VTXO and it's not the wanted asset, both the swap-VTXO group and the collateral group would route it to output 1. Should be fine since they're separate AssetGroup entries, but worth a sanity check that the server/introspector handles overlapping groups.

4. Number(offer.wantAmount) in AssetOutput.create — wantAmount is a bigint but AssetOutput.create takes number. Safe for typical amounts but could silently lose precision for values > Number.MAX_SAFE_INTEGER. Not a realistic concern for sats but worth noting.

5. Test uses fulfillByTxid path removed — The BTC swap test was changed from taker.fulfill(fundingTxid) to taker.fulfill(offerHex). The fulfillByTxid path still exists but now lacks E2E coverage.

Overall: solid fix, the asset fulfillment script correction was critical, and the coin selection rewrite brings the taker up to parity with the wallet's send logic.

Notes on the partial fill script from @msinkec:

Generalisation

The contract does not necessarily need to always be funded with BTC, it could be selling ArkadeAssets for BTC or ArkadeAsset for ArkadeAsset.

The script builder looks at the offerAsset and quoteAsset, then adjusts Step 2 and Step 4 according to the asset type.

type SwapAsset = "btc" | asset.AssetId

export interface BancoSwapParams {
    offerAmount: bigint,
    offerAsset: SwapAsset,
    quoteAmount: bigint,
    quoteAsset: SwapAsset,
	// ...
}

export class BancoSwap {
    constructor(
        readonly params: BancoSwapParams,
		// ...
    ) {}

	partialFillScript(): Uint8Array {
		const verifyAssetLookup = [
			"OP_DUP", "OP_1NEGATE", "OP_EQUAL"
			"OP_NOT", "OP_VERIFY"
		];
		
		// ...
		// Construct Step 2:
		const readReceivedQuoteAssetAmount = () => {
			if (params.quoteAsset === "btc") {
				return [
					"OP_1",
					"OP_INSPECTOUTPUTVALUE"
				]
			}
			return [
				"OP_1",
				params.quoteAsset.txid,
				params.quoteAsset.groupIndex,
				"OP_INSPECTOUTASSETLOOKUP",
				...verifyAssetLookup
			]
		}
		// ...
		// Construct Step 4:
		const readAvailableOfferAssetAmount = () => {
			if (params.quoteAsset === "btc") {
				return [
					"OP_PUSHCURRENTINPUTINDEX",
					"OP_INSPECTINPUTVALUE"
				]
			}
			return [
				"OP_PUSHCURRENTINPUTINDEX",
				params.quoteAsset.txid,
				params.quoteAsset.groupIndex,
				"OP_INSPECTINASSETLOOKUP",
				...verifyAssetLookup
			]
		}
	}
}

The hardcoded ratio becomes:

Product Overflow

In order to compute consumed, the received amount must be first multiplied by the ratioNum to compute intermediateProduct, where intermediateProduct <= 2^64 (1.8446744074E19) or the script will fail when verifying the overflow flag.

Example:

offerAsset = "btc"
offerAssetAmount = 10 000 000 000 sats (100 BTC)

quoteAsset = X asset
quoteAssetAmount = 2 000 000 000 (asset with 9 decimals)

takerInput = 2 000 000 000 X (full fill attempt)

intermediateProduct = 2 000 000 000 * 10 000 000 000 = 2E19 (overflow)

Mitigations:

• Simplify ratio components prior to encoding it within the script:

   // compute greatest common denominator between numerator and denominator
   g = gcd(ratioNum, ratioDen)
   // reducde the size of numerator and denominator to be encoded in the script
   ratioNum' = ratioNum / g
   ratioDen' = ratioDen / g
  
  
   // Applied to the example above:
   ratioNum  = 10 000 000 000
   ratioDen  =  2 000 000 000
          g  =  2 000 000 000
   ratioNum' = 10 000 000 000 / 2 000 000 000 = 5
   ratioDen' =  2 000 000 000 / 2 000 000 000 = 1
  
   // Partial fill 100 000 X
   consumed = (100 000 * 5) / 1 = 500 000 / 1 = 500 000
            = (100 000 * 10 000 000 000) / 2 000 000 000 = 1E15 / 2 000 000 000 = 500 000 
  
   // Full fill 2 000 000 000 X
   consumed = (2 000 000 000 * 5) / 1  = 10 000 000 000 / 1 = 10 000 000 000
            = (2 000 000 000 * 10 000 000 000) / 2 000 000 000 = (overflow err)
  

• Orders with large numerators and/or denominators without a simplifying GCD can be partially filled up the the overflow limit.

• Add op codes for extra wide multiplication and division, e.g. OP_MUL128 takes 2 8-16 byte little endian integers off the stack, converts to u128, multiplies, and places the 16 byte result on the stack. Similar for OP_DIV128. The 16 byte result can be split and the high 64 bits are checked to equal 0 (no conversion overflow) and discarded.

Ratios less than 1

If the maker wants more atomic units of the quoteAsset than the number of atomic units of offerAsset, then (ratioNum / ratioDen) < 1.

This works fine but introduces a minFillAmount due to the flooring integer division.

The taker should always be expecting: consumed >= 1 atomic unit.

Example:

offerAsset = X asset
offerAssetAmount = 5 X

quoteAsset = "btc"
quoteAssetAmount = 100 000 sats

ratioNum  =       5
ratioDen  = 100 000
       g  =       5
ratioNum' = 5 / 5 = 1
ratioDen' = 100 000 / 5 = 20 000

takerInput = 19 999 sats
  consumed = (19 999 * 1) / 20 000 = 0 (script should fail to protect taker)

minFillAmount = ciel(ratioDen / ratioNum) = 20 0000 / 1 = 20 000

// The taker should supply the quote asset in multiples of `minFillAmount` to avoid over-payment
takerInput = 39 999 sats
  consumed = (39 999 * 1) / 20 000 = 1  // (if taker allows receiving 1 X, they over-paid by 19 999 sats)

This is an unavoidable UX challenge due to the atomic nature of the units.

Subdust outputs

arkd will currently not co-sign the partial-fill transaction if:

• Any output has a subdust value that is not an OP_RETURN based subdust script

• Any asset references an OP_RETURN output that is NOT a subdust script

Scenario 1: offerAsset == "btc"

offerAsset = "btc"
offerAssetAmount = 100 000 sats

quoteAsset = X Asset
quoteAssetAmount = 5 000 000 X

ratioNum  =   100 000
ratioDen  = 5 000 000
       g  =   100 000
ratioNum' = 100 000 / 100 000 = 1
ratioDen' = 5 000 000 / 100 000 = 50

minFillAmount = ceil(50 / 1) = 50

takerInput = quoteAssetAmount - minFillAmount = 5 000 000 - 50 = 4 999 950
consumed = (4 999 950 * 1) / 50 = 99 999
change = 100 000 - 99 999 = 1 (invalid value for recursive output)

Either the script should enforce that a taker can only leave >=dustLimit sats in the contract (which is implicitly enforced by arkd) or if the change < dustLimit, allow the taker to close the contract by attributing a sub-dust output to the maker.

It seems better to enforce that the taker must fully fill rather than force the make to accept a subdust output.

The other consideration is the asset output that transfers received ArkadeAsset to the maker.

As long as the maker specifies a non-subdust script as the maker witness program, it will need to have >=dustLimit sats (also implicitly enforced by arkd).

Scenario 2: offerAsset == ArkadeAsset, quoteAsset == "btc"

In this case the taker is sending the maker sats in exchange for ArkadeAssets. arkd will enforce that:

• The taker attributes >=dustLimit sats to any change left in the recursive output in the case of a partial fill.
• received >= dustLimit if the maker specified a non-subdust script as the maker witness program.
• It is up to the taker whether they receive the consumed ArkadeAssets on a subdust script or not.

Scenario 3: offerAsset == ArkadeAsset, quoteAsset == ArkadeAsset

In this case the taker is sending the maker ArkadeAssets in exchange for other ArkadeAssets. arkd will enforce that:

• The taker attributes >=dustLimit sats to any change left in the recursive output in the case of a partial fill.
• The value attributed to the received ArkadeAsset output if >=dustLimit if the maker specified a non-subdust script as the maker witness program.
• It is up to the taker whether they receive the consumed ArkadeAssets on a subdust script or not.

Open Question: How much should ArkadeScript's rely on the current implementations of co-signers (e.g. arkd, introspector) to simplify contract logic?

Open Question: How much should ArkadeScript's rely on the current implementations of co-signers (e.g. arkd, introspector) to simplify contract logic?

For dust related issues I think we can lean on arkds limitations.

@louisinger Opened a PR for partial swaps: louisinger#5

🔍 Review — 0273712e: banco partial-fill swaps

New commit implements partial-fill covenant scripts for three swap directions (BTC→asset, asset→BTC, asset→asset) plus the corresponding taker output-building logic.

Security: Zero-consumed partial fills

The test contract.test.ts explicitly acknowledges: computeConsumed(1n, 1n, 1000n) === 0n. When a taker delivers a tiny amount where fillAmount * ratioNum / ratioDen floors to zero, the script allows a transaction where:

• Change output equals full input value (swap VTXO unchanged)
• Maker receives a dust amount of asset "for free"

The scripts do not enforce consumed > 0. While this isn't a theft vector (the swap VTXO BTC is preserved), it enables griefing: a taker can submit no-op transactions consuming introspector + server co-signing resources. Consider adding a consumed > zeroLE64 / GREATERTHAN64 / VERIFY check after the DIV64 in each partial-fill script, or document that the introspector is expected to reject zero-consumed fills.

Covenant script correctness

Full-fill branches: All three directions end the IF branch with a truthy stack value (1 for btcForAsset, 1 for assetForBtc, 1 for assetForAsset) — correct for script evaluation.

Partial-fill branches (ELSE): Each correctly reconstructs change by SUB64 of inputValue - consumed, then verifies output 0 preserves the swap VTXO's scriptPubKey and value/asset balance. The PUSHCURRENTINPUTINDEX / INSPECTINPUTSCRIPTPUBKEY → INSPECTOUTPUTSCRIPTPUBKEY comparison ensures the change VTXO stays in the same contract — good.

assetForBtcScript partial-fill branch: After asset change verification, the script uses a third FROMALTSTACK to recover the offer txid for INSPECTOUTASSETLOOKUP. Three TOALTSTACK pushes (two DUP + TOALTSTACK in Step 3) match three pops (Steps 4, 5/6 via FROMALTSTACK) — stack bookkeeping looks correct.

Taker output layout

The taker correctly distinguishes 6 output layouts:

• Full-fill BTC→asset: [taker, maker]
• Partial BTC→asset: [bancoChange, maker, taker]
• Full-fill asset→BTC: [makerDustReturn, maker, taker]
• Partial asset→BTC: [bancoChange, maker, taker]
• Legacy full-fill (no ratio): [maker, takerSwap, takerChange?]
• asset→asset: follows the asset→BTC pattern

The makerOutputIdx = 1 constant is consistent across partial-fill paths. The collectCollateral helper properly excludes both want and offer asset IDs from collateral routing.

Offer TLV encoding

New fields TLV_RATIO_NUM (0x09), TLV_RATIO_DEN (0x0a), TLV_OFFER_ASSET (0x0b) follow the existing pattern. The GCD reduction in maker.ts ensures minimal encoding.

Minor nits

• src/utils/math.ts — gcd handles negative inputs correctly via abs, but ratioNum/ratioDen are validated as positive in maker.createOffer. The negative handling is defensive but the validation makes it unreachable.
• The bigintToLE64 in contract.ts uses setBigInt64 (signed). For ratio values this is fine since they're positive and fit in 63 bits, but setBigUint64 would be more semantically correct for unsigned ratio values.

Cross-repo

The introspector (ArkLabsHQ/introspector) co-signs these scripts via the same ArkScriptHash tweak mechanism — no introspector changes needed for partial fills since it validates the covenant script at intent registration time. The covenant change is purely client-side.

Overall solid implementation. The zero-consumed griefing vector is the main open question — worth a decision on whether to enforce in-script or at the service layer.

🔍 Arkana PR Review — #319

Arkade script support — a major feature PR (6219+/205-, 51 files) by @louisinger adding client-side Arkade Script support, introspector integration, Banco swap contracts, and comprehensive tests.

Architecture Overview

The PR adds five major subsystems:

1. src/arkade/ — Arkade opcodes, script codec, key tweaking, VTXO script extension, batch handler
2. src/banco/ — Peer-to-peer atomic swap system (Maker/Taker/Offer/BancoSwap contract)
3. src/providers/introspector.ts — REST client for the introspector co-signing service
4. src/extension/introspector/ — IntrospectorPacket (type 0x01) for PSBT extension fields
5. src/extension/utils.ts — Refactored BufferReader/BufferWriter with CompactSize support

Security Observations

✅ Good: PSBT field key matching fix (unknownFields.ts)

The change from checkKeyIncludes (string .includes()) to checkKeyMatch (byte-level prefix/exact matching) is a genuine security fix. The old code could match if the expected key bytes appeared anywhere in the key data, not just at the correct position. The new implementation correctly checks byte-by-byte from the start:

// Before (unsafe): hex-encoded string .includes() — could match substring anywhere
// After (correct): byte-level comparison from index 0

This also fixes the nullIfCatch to catch without binding the unused error variable — clean.

⚠️ Note: Hardcoded introspector secret key in docker-compose.yml

INTROSPECTOR_SECRET_KEY=b1c2d3e4f5a6b7c8d9e0f1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c4

Fine for regtest/dev, but worth a comment in the file to prevent copy-paste into production.

✅ Good: Key tweaking (tweak.ts)

The computeArkadeScriptPublicKey implementation correctly:

• Forces even-Y via BIP-340 lift_x convention (matching the Go introspector)
• Reduces the hash scalar modulo curve order n
• Falls back to 1n if reduction yields 0n (avoiding point-at-infinity, though astronomically unlikely)

⚠️ Partial fill consumed=0 edge case (banco/contract.ts)

When ratioNum/ratioDen yields a very small ratio, a small fillAmount can produce consumed = 0 via integer floor division:

// Example: computeConsumed(1n, 1n, 1000n) === 0n

This is tested and acknowledged in contract.test.ts, but the Taker code doesn't explicitly guard against submitting a zero-consumed partial fill. The introspector/server might reject it, but a client-side check would be safer.

✅ Good: Offer TLV validation

The Offer.decode() properly validates:

• Required fields (swapAddress, wantAmount, makerPkScript, makerPublicKey, introspectorPubkey)
• Field lengths (34 bytes for pkScript, 32 bytes for pubkeys)
• Rejects unknown TLV types
• Rejects truncated data

Protocol Correctness

✅ Covenant scripts

The fulfill script correctly verifies:

• Output 0 scriptPubKey matches maker's taproot address
• Output 0 value ≥ wantAmount (BTC case)
• Asset lookup + amount check (asset case)

The partial fill scripts handle three directions (BTC→asset, asset→BTC, asset→asset) with proper branching between full-fill and partial-fill paths, including change output verification and scriptPubKey preservation.

⚠️ Taker address reconstruction verification

Good practice — the Taker verifies swapPkScript === expectedPkScript before proceeding:

if (swapPkScript !== expectedPkScript) {
    throw new Error("Offer inconsistency: swapAddress does not match the reconstructed contract");
}

This prevents a malicious offer from directing funds to a wrong contract.

Code Quality

✅ BufferReader/BufferWriter refactoring

Clean extraction from extension/asset/utils.ts to extension/utils.ts. All existing imports updated. CompactSize methods added for IntrospectorPacket wire format.

⚠️ Expo export change (src/worker/expo/index.ts)

-export { runTasks, createTaskDependencies } from "./taskRunner";
+    runTasks,
 } from "./taskRunner";

runTasks moved to a type-only export and createTaskDependencies export was removed. This is a breaking change for consumers importing runTasks as a value or createTaskDependencies at all. If intentional, it should be noted in the PR description.

✅ Test coverage

Excellent test coverage:

• 696-line unit test suite for arkade opcodes, script encoding, ASM conversion
• IntrospectorPacket fixture-based tests
• Banco contract unit tests (covenant dispatch, ratio arithmetic, GCD reduction)
• Offer TLV round-trip tests with edge cases
• E2E tests: boarding flow, settlement flow, settlement with assets, banco BTC-for-asset and asset-for-asset swaps

Cross-repo Impact

This PR adds the client-side counterpart to ArkLabsHQ/introspector. The opcode definitions in src/arkade/opcodes.ts mirror introspector/pkg/arkade/opcode.go. The tagged hash tag "ArkScriptHash" and tweak computation must stay in sync between the two repos.

The Banco swap system depends on the introspector's /v1/tx, /v1/intent, and /v1/finalization REST endpoints.

Summary

A well-structured, well-tested feature PR. The key areas to address before merge:

1. Expo export breaking change — confirm intentionality of runTasks/createTaskDependencies export changes
2. Zero-consumed partial fill — consider a client-side guard
3. Docker secret key — add a comment noting it's dev-only

No blocking security issues found. The PSBT field matching fix is a nice improvement.

🔍 Incremental Review — #319 (new commit: 1a8c6e77)

Commit: "cleaning"

What Changed

Consolidation of the banco module — removes the standalone example app and test files, merges contract logic into the offer/maker/taker files:

• Removed: examples/banco/ (standalone example app + scripts)
• Removed: src/banco/contract.ts (576 lines) — logic absorbed into offer.ts
• Removed: test/unit/banco/contract.test.ts, test/unit/banco/offer.test.ts
• Modified: src/banco/offer.ts (+520/-23) — now contains the full covenant script generation (fulfill, partial-fill, cancel/exit tapscript building) previously in contract.ts
• Modified: src/banco/maker.ts (+20/-58) — simplified to use Offer.vtxoScript() and Offer.covenantScript() instead of separate contract helpers
• Modified: src/banco/taker.ts (+6/-17) — same consolidation
• Modified: test/e2e/banco.test.ts (+144/-10) — expanded E2E coverage including asset-for-asset swap test
• Modified: test/extension.test.ts — minor import path adjustments

Assessment

Good cleanup — reduces module surface area by consolidating contract.ts into the Offer namespace where the TLV encoding already lives. The Offer.vtxoScript() and Offer.covenantScript() functions provide a cleaner API than having a separate contract builder.

Unit tests for offer/contract were removed, but E2E tests were expanded. The removed unit tests covered encoding/decoding and script construction which is now exercised through the E2E banco tests. Reasonable tradeoff for an alpha-stage module, though dedicated unit tests for the covenant script generation in Offer would add safety once the partial-fill scripts stabilize.

No protocol-level concerns with this cleanup commit.

Banco should be a separated module I think as is not part of Arkade being application layer.

🔍 Arkana PR Review — arkade-os/ts-sdk#319 (Arkade Script Support)

Scope: +5559/−205 across 44 files. Major feature: full client-side Arkade script support with introspector co-signing, Banco swap system, and PSBT field improvements.

Architecture

Clean modular structure: arkade/ (opcodes, script codec, tweak, batch handler, vtxoScript), banco/ (offer TLV, maker, taker), providers/introspector.ts, extension/introspector/. The separation between script-level primitives and higher-level swap logic is well done.

Security Analysis

1. Tweak computation (tweak.ts)
The computeArkadeScriptPublicKey does P + H("ArkScriptHash", script) * G which matches the Go introspector's behavior. Key observations:

• ✅ Forces even-Y via lift_x ("02" + xOnly) — matches Go's schnorr.ParsePubKey(schnorr.SerializePubKey()) round-trip
• ✅ Uses BIP-340 tagged hash
• ⚠️ Scalar reduction: bytesToBigInt(hash) % n || 1n — the || 1n fallback handles the astronomically unlikely case where the hash reduces to 0 mod n. This is correct but worth a comment explaining why.

2. Batch handler (batch.ts)

• ✅ Correctly validates intent inclusion via SHA256 hash comparison before confirming registration
• ✅ Forfeit transactions use SigHash.DEFAULT on the VTXO input — correct for tapscript spends
• ✅ Boarding inputs get signed on the commitment tx; settlement inputs get forfeit txs — clean separation
• ✅ Connector tree nodes are properly constructed for introspector finalization

3. Banco covenant scripts (offer.ts)
The covenant scripts are complex but follow sound patterns:

• Full-fill: checks output scriptPubKey matches maker + output value ≥ wantAmount
• Partial-fill: ratio enforcement via MUL64/DIV64, change output validation with scriptPubKey preservation
• ⚠️ Partial fill scripts lack explicit overflow protection on MUL64. If fillAmount * ratioNum overflows 64-bit, the introspector's behavior depends on whether MUL64 returns an error flag. The "VERIFY" after MUL64 should catch this as MUL64 pushes a success flag, but worth confirming against the introspector opcode spec.

4. PSBT field matching (unknownFields.ts)

• ✅ Good fix: checkKeyIncludes → checkKeyMatch with exact byte comparison. The old hex.encode().includes() was a substring match which could false-positive on keys that happen to contain the expected bytes at non-zero offsets. This is a correctness improvement.
• ✅ prefixOnly mode for CosignerPublicKey where the key has an appended index byte — correct.
• ✅ v !== null instead of truthy check — fixes the case where decoded value is 0 or empty array.

5. Docker compose

• ⚠️ INTROSPECTOR_SECRET_KEY is hardcoded in docker-compose.yml. Fine for local dev/testing, but add a comment marking it as test-only to prevent accidental production use.

Protocol Correctness

Opcode mapping (opcodes.ts):
Cross-referenced against arkade-os/introspector pkg/arkade/opcode.go:

• Range 0xc4–0xf3 matches the Go implementation
• 0xb3 (MERKLEBRANCHVERIFY) correctly maps to repurposed NOP4 slot
• ⚠️ Missing opcodes check: INSPECTINPUTASSETID (0xc8) and INSPECTOUTPUTNONCE (0xd0) are absent from the mapping. If these exist in the Go implementation, they should be included even if unused, to avoid confusion when decoding scripts that use them.

TLV offer format:

• Encoding is consistent (1B type + 2B BE length + value)
• wantAmount/cancelDelay/ratios use big-endian uint64 — verify this matches any Go counterpart
• exitTimelock uses 1B type flag + 8B BE value — clean

Cross-Repo Impact

• arkade-os/introspector: The REST API paths (/v1/info, /v1/tx, /v1/intent, /v1/finalization) must match. The submitFinalization body uses signedIntent (not intent) — this is intentional per the comment, but worth a cross-check.
• ArkLabsHQ/introspector#49 (LE64 vs scriptNum fix): The opcode usage in covenant scripts should be verified against the patched introspector behavior.

Minor Items

1. src/utils/math.ts adds gcd() — simple Euclidean algorithm, correct.
2. src/extension/utils.ts consolidates BufferReader/BufferWriter from asset/utils.ts — good refactor, reduces duplication.
3. The Recipient type gains an extensions field for custom extension packets in wallet/index.ts — backwards compatible.
4. pnpm-workspace.yaml adds examples/banco workspace — clean.
5. Test coverage looks comprehensive: unit tests for opcodes/script/tweak, E2E for boarding/settlement/banco flows.

Summary

Solid, well-structured implementation. The main areas to double-check:

1. MUL64 overflow behavior in partial-fill scripts against introspector spec
2. Missing opcode mappings (0xc8, 0xd0) if they exist in the Go implementation
3. REST API contract alignment with introspector service

No blocking issues found.

🔍 Arkana PR Review — ts-sdk#319

PR: Arkade script support

Summary

Major feature PR adding client-side support for ArkadeScript (introspector co-signing service). Includes: Arkade opcodes & script codec, public key tweaking, ArkadeVtxoScript, introspector REST client, batch handler for boarding + settlement flows, banco (maker/taker swap protocol), PSBT field improvements, and comprehensive tests.

Security Review

✅ Key tweaking implementation (critical — correct)
computeArkadeScriptPublicKey in tweak.ts computes P + taggedHash("ArkScriptHash", script) * G. Uses BIP-340 tagged hash via schnorr.utils.taggedHash. The scalar is reduced modulo curve order n with || 1n fallback to avoid zero. The x-only round-trip matches the Go introspector's behavior. This is sound.

✅ PSBT field matching tightened (security improvement)
unknownFields.ts replaces checkKeyIncludes (hex substring match) with checkKeyMatch (exact byte comparison). The old approach could match keys where the expected bytes appeared as a substring of a longer key. Now uses exact length matching, with prefixOnly flag only for CosignerPublicKey (which has a variable-length suffix for the index byte). Good hardening.

✅ Introspector provider validates responses
REST client in providers/introspector.ts validates all response fields: checks signerPubkey is a non-empty string, signedArkTx is present, signedForfeits is an array of strings, etc. Throws on unexpected responses. Sound defensive coding.

✅ Batch handler forfeit signing (critical path — correct)
batch.ts correctly separates boarding inputs (signed on commitment tx) from settlement inputs (signed via forfeit txs). Forfeit outputs go to forfeitAddress from server info. The handler properly passes connector tree nodes to the introspector for finalization.

⚠️ Introspector secret key in docker-compose
docker-compose.yml hardcodes INTROSPECTOR_SECRET_KEY=b1c2d3.... This is fine for local dev/testing but should be flagged — ensure this isn't used in any non-local environment. Consider adding a comment.

Observations

1. Banco offer TLV format — The offer namespace defines a clean TLV wire format. The fromHex/toHex round-trip is tested. One note: the swapPkScript field in Data is first set to an empty Uint8Array(0) as placeholder, then computed. This works but is fragile — if someone serializes the offer before vtxoScript() is called, they'd get an empty pkScript.

2. Partial-fill covenant script — The ratio-based partial fill script uses LE64TOSCRIPTNUM and MUL64/DIV64 for price math. The overflow behavior of 64-bit multiplication should be validated against the introspector's implementation to ensure matching semantics.

3. bytesToBigInt in tweak.ts — Manual big-endian conversion. Works correctly for 32-byte hash outputs. No overflow concern since it's used as scalar input and immediately reduced modulo n.

4. BufferReader/BufferWriter — Clean implementations with proper bounds checking ("unexpected end of buffer" on overread). CompactSize decoding rejects 8-byte values explicitly (throw). Good.

5. Extension packet injection — The wallet changes allow recipients to include custom extensions packets. These are appended after the asset packet. No validation on ext.type — callers could inject packets with conflicting types. The consumer (server/indexer) would need to handle duplicates.

6. Server Dockerfile branch change — BRANCH=v0.9.0 → BRANCH=v0.9.0-rc.4 — this pins to a release candidate. Make sure this is intentional and doesn't accidentally ship to CI that builds production images.

7. pnpm-workspace.yaml addition — Adds examples/banco as a workspace member. Verify that this doesn't affect the published npm package contents.

Cross-repo Impact

• Depends on: ArkLabsHQ/introspector (REST API at /v1/info, /v1/intent, /v1/finalization, /v1/tx)
• Opcode definitions must stay in sync with arkade-os/introspector pkg/arkade/opcode.go
• Banco protocol: New capability that taker/maker implementations will reference. The wire format should be considered semi-stable once merged.

Verdict

Well-structured feature addition. The security-critical paths (key tweaking, PSBT field matching, forfeit signing) are correctly implemented. The introspector client has proper response validation. Main items to watch: hardcoded secret key in docker-compose (dev-only), and ensuring opcode definitions stay synced with the Go introspector.

Review by Arkana 🔍 — [Agent: pr-reviewer]

🔍 Arkana PR Review — #319 (Arkade script support)

Scope: +5,677 / -217 across 48 files. Adds full client-side Arkade script support: opcodes, script codec, key tweaking, introspector provider, batch handler, banco swap maker/taker, PSBT field improvements, extension refactoring, and comprehensive tests.

✅ Strengths

1. Clean opcode registry (src/arkade/opcodes.ts): Bidirectional mappings with getOpcodeName/getOpcodeValue helpers. Covers the full range 0xb3, 0xc4–0xf3 matching the Go introspector reference.

2. ArkadeScript CoderType (src/arkade/script.ts): Correctly extends the @scure/btc-signer Script pattern. The encoder/decoder handles standard Bitcoin data pushes (OP_0, OP_1–OP_16, PUSHDATA1/2/4) alongside Arkade extension opcodes. Round-trip fidelity is validated by tests.

3. Key tweaking (src/arkade/tweak.ts): Uses BIP-340 tagged hash with "ArkScriptHash" tag, matches the Go introspector's lift_x → even-Y convention. The computeArkadeScriptPublicKey correctly forces even Y via "02" + xOnly before point addition.

4. ArkadeVtxoScript (src/arkade/vtxoScript.ts): Elegant design — extends VtxoScript, auto-tweaks introspector keys before encoding into taproot tree, and tracks arkade leaf indices via ReadonlyMap. The processScripts helper is well-factored.

5. Batch handler (src/arkade/batch.ts): Correctly handles the dual boarding+settlement flow. Properly separates boarding inputs (signed on commitment tx) from settlement inputs (forfeit txs via connectors). The introspector finalization step is correctly sequenced after user signing.

6. PSBT field matching fix (src/utils/unknownFields.ts): The checkKeyIncludes → checkKeyMatch rename with prefixOnly flag is a real bug fix — the old .includes() on hex strings could match partial key overlaps. The v !== null check (replacing truthy v) correctly allows zero/empty values.

7. Extension refactoring: Moving BufferReader/BufferWriter to src/extension/utils.ts with CompactSize support, adding IntrospectorPacket and BancoOffer packet types — clean architecture.

8. Test coverage: 776-line unit test suite covers opcodes, encoding, ASM, VtxoScript construction, witness hashing. E2E tests against local Ark+introspector stack.

⚠️ Concerns & Questions

Security

1. Hardcoded introspector secret key in docker-compose (docker-compose.yml:107):

   INTROSPECTOR_SECRET_KEY=b1c2d3e4f5a6b7c8d9e0f1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c4
   

   This is fine for local dev/testing. Just ensure this key is never referenced in production configs or documentation as a default. Consider adding a comment like # DEV ONLY — DO NOT USE IN PRODUCTION.

2. Scalar reduction in computeArkadeScriptPublicKey (src/arkade/tweak.ts:67):

   const scalar = bytesToBigInt(hash) % secp256k1.Point.CURVE().n || 1n;

   The || 1n fallback handles the astronomically unlikely case where hash mod n == 0, which would produce the point at infinity. This is correct and defensive. However, the Go introspector should have equivalent handling — have you verified that the Go side also handles this edge case identically? A mismatch here would cause signing failures.

3. Banco partial-fill scripts — integer division truncation: In src/banco/taker.ts:198:

   const consumed = (fillAmount * offer.ratioNum!) / offer.ratioDen!;

   BigInt division truncates. The introspector's script verification must use the same truncation semantics. Is this confirmed to match the LE64 DIV64 opcode behavior in the introspector VM?

4. No nonce reuse protection in batch handler: The onTreeNonces handler calls session.sign() after aggregating nonces. If the same session object is reused across multiple batch attempts (e.g., server restarts a round), this could lead to nonce reuse in MuSig2. Is the SignerSession designed to be single-use? Worth a comment if so.

Protocol Correctness

5. VtxoScript.pkScript change (src/script/base.ts):
   The getter get pkScript() was replaced with a readonly property set in the constructor:

   this.pkScript = payment.script;

   This is semantically equivalent (both produce the same P2TR scriptPubKey) but uses @scure/btc-signer's computed payment.script instead of manually encoding [OP_1, tweakedPubkey]. This is correct and cleaner. Just noting it's a behavioral change that downstream consumers should be aware of.

6. Offer TLV encoding uses big-endian while the covenant scripts use LE64 for ratio values. This is internally consistent (wire format ≠ script format), but worth documenting to avoid future confusion.

7. assetForBtcScript and assetForAssetScript — these partial-fill scripts are complex (~100 opcodes each). They need to be verified against the introspector's expected execution model. Are there specific test vectors from the Go side that validate these scripts produce identical results?

Nits

8. src/worker/expo/index.ts: The diff shows runTasks moved from a regular export to a type export position. If runTasks is a runtime function (not just a type), this would be a breaking change for consumers.

9. server.Dockerfile:8: Branch changed from v0.9.0 to v0.9.0-rc.4. This is fine for testing but should be bumped to a stable release before merging to master.

10. FULMINE_DELEGATOR_FEE=0 removed from docker-compose. Intentional? This changes default behavior for the local dev stack.

🔗 Cross-Repo Impact

• arkade-os/introspector: The opcodes in opcodes.ts must stay in sync with pkg/arkade/opcode.go. The tagged hash tag strings ("ArkScriptHash", "ArkWitnessHash") must match exactly.
• ArkLabsHQ/introspector: REST API endpoints (/v1/info, /v1/tx, /v1/intent, /v1/finalization) and their request/response schemas must be aligned.
• arkade-os/wallet: The Recipient.extensions field addition and ExtensionPacket plumbing in wallet.ts are additive and backward-compatible.

Summary

This is a well-structured, significant feature addition. The core cryptographic operations (key tweaking, script encoding) look correct. The main risks are in the complex banco partial-fill scripts where any divergence from the introspector's execution semantics would cause silent failures. I'd recommend cross-validating the partial-fill script test vectors against the Go introspector test suite before merging.

The PSBT checkKeyMatch fix is a genuine security improvement worth highlighting.

Overall: strong work, needs cross-repo validation of the covenant scripts.